-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Small fixes to PlatformHelper.php #218
Conversation
Conditional checks for existence of 'getNativeConnection' method, but then it doesn't call that method at all, and attempts to call directly `$connection->getServerVersion()`, which would fail because that method is private.
…may not have a patch version, in which case $versionParts will be `[]`.
@@ -104,11 +104,11 @@ public static function getOracleMysqlVersionNumber(string $versionString): strin | |||
public static function getMariaDbMysqlVersionNumber(string $versionString): string | |||
{ | |||
preg_match( | |||
'#^(?:5\.5\.5-)?(mariadb-)?(?P<major>\d+)\.(?P<minor>\d+)\.(?P<patch>\d+)#i', | |||
'#^(?:5\.5\.5-)?(mariadb-)?(?P<major>\d+)\.(?P<minor>\d+)(\.(?P<patch>\d+))?#i', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I assume you're right even if this regex comes from doctrine/dbal
itself: https://github.com/doctrine/dbal/blob/7a8252418689feb860ea8dfeab66d64a56a64df8/src/Driver/AbstractMySQLDriver.php#L96
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just got the error when I was running tests.
$versionString
, as reported by a local postrgres running in a container, was "13.16"
That string matches what I'd expect when checking postgres releases:
https://www.postgresql.org/docs/release/
Maybe the problem is that this function was called to parse a postgres version, and it should be reserved for a MariaDb version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I changed the regex I was able to run my project's tests again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, on further inspection, I think the problem is that this version-parsing is being called when not needed. I believe the problem may come from isJsonSupported()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, isJsonSupported()
seems to be the culprit, will post a fix soon.
Would you mind dumping $version
after
$version = self::getServerVersion($connection); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I hadn't understood you got the version that way ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#219 should fix your issues
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, that works. thx.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will release it asap
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…nString may not have a patch version, in which case $versionParts will be `[]`." This reverts commit 3e27a78.
$connection->getServerVersion()
, which would fail because that method is private.fix getMariaDbMysqlVersionNumber(), because sometimes $versionString may not have a patch version, in which case $versionParts will be[]
.